Skip to content

fix(compliance): add /get_feature_flag endpoint to the test adapter#542

Open
turnipdabeets wants to merge 10 commits into
mainfrom
fix/compliance-adapter-get-feature-flag
Open

fix(compliance): add /get_feature_flag endpoint to the test adapter#542
turnipdabeets wants to merge 10 commits into
mainfrom
fix/compliance-adapter-get-feature-flag

Conversation

@turnipdabeets
Copy link
Copy Markdown
Contributor

@turnipdabeets turnipdabeets commented May 29, 2026

TL;DR: the compliance adapter was missing /get_feature_flag → harness 404s → 0/16 flag tests. This adds the endpoint (mirrors posthog-ios).

turnipdabeets and others added 2 commits May 29, 2026 11:43
The harness was getting a 404 on /get_feature_flag, failing all 16
feature-flag tests. Implements the endpoint (mirrors posthog-ios): set
person/group properties, reload flags, resolve the value, flush.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Satisfies the semgrep package-manager rules: add a 7-day cooldown to the
Dependabot github-actions ecosystem and minimumReleaseAge: 10080 to the
pnpm workspace, so newly published packages wait before being adopted.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 29, 2026

posthog-android Compliance Report

Date: 2026-06-03 15:25:20 UTC
Duration: 119784ms

⚠️ Some Tests Failed

38/45 tests passed, 7 failed


Capture Tests

⚠️ 28/29 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 396ms
Format Validation.Event Has Uuid 36ms
Format Validation.Event Has Lib Properties 32ms
Format Validation.Distinct Id Is String 30ms
Format Validation.Token Is Present 30ms
Format Validation.Custom Properties Preserved 32ms
Format Validation.Event Has Timestamp 36ms
Retry Behavior.Retries On 503 7031ms
Retry Behavior.Does Not Retry On 400 4029ms
Retry Behavior.Does Not Retry On 401 4025ms
Retry Behavior.Respects Retry After Header 7030ms
Retry Behavior.Implements Backoff 17035ms
Retry Behavior.Retries On 500 7020ms
Retry Behavior.Retries On 502 7019ms
Retry Behavior.Retries On 504 7022ms
Retry Behavior.Max Retries Respected 17033ms
Deduplication.Generates Unique Uuids 36ms
Deduplication.Preserves Uuid On Retry 7016ms
Deduplication.Preserves Uuid And Timestamp On Retry 12030ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 7019ms
Deduplication.No Duplicate Events In Batch 34ms
Deduplication.Different Events Have Different Uuids 26ms
Compression.Sends Gzip When Enabled 19ms
Batch Format.Uses Proper Batch Structure 17ms
Batch Format.Flush With No Events Sends Nothing 13ms
Batch Format.Multiple Events Batched Together 31ms
Error Handling.Does Not Retry On 403 4018ms
Error Handling.Does Not Retry On 413 4014ms
Error Handling.Retries On 408 5026ms

Failures

error_handling.does_not_retry_on_413

Expected 1 requests, got 2

Feature_Flags Tests

⚠️ 10/16 tests passed, 6 failed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 34ms
Request Payload.Flags Request Uses V2 Query Param 21ms
Request Payload.Flags Request Hits Flags Path Not Decide 22ms
Request Payload.Flags Request Omits Authorization Header 21ms
Request Payload.Token In Flags Body Matches Init 21ms
Request Payload.Groups Round Trip 22ms
Request Payload.Groups Default To Empty Object 27ms
Request Payload.Person Properties Distinct Id Auto Populated When Caller Omits It 25ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 37ms
Request Payload.Disable Geoip Omitted Defaults To False 25ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 27ms
Request Lifecycle.No Flags Request On Init Alone 10ms
Request Lifecycle.No Flags Request On Normal Capture 26ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 2026ms
Request Lifecycle.Mock Response Value Is Returned To Caller 19ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 22ms

Failures

request_payload.request_with_person_properties_device_id

Expected distinct_id='test_user_123', got '019e8e16-c5d7-70c7-a414-52ed4cae5c27'

request_payload.groups_default_to_empty_object

Field 'groups' not found in /flags request body at path 'groups'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']

request_payload.person_properties_distinct_id_auto_populated_when_caller_omits_it

Field 'distinct_id' not found in /flags request body at path 'person_properties.distinct_id'. Available keys: ['$lib', '$lib_version', 'email']

request_payload.disable_geoip_false_propagates_as_geoip_disable_false

Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']

request_payload.disable_geoip_omitted_defaults_to_false

Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']

request_payload.flag_keys_to_evaluate_contains_only_requested_key

Field 'flag_keys_to_evaluate' not found in /flags request body at path 'flag_keys_to_evaluate'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']

…n init

Declare capabilities ["capture_v0", "encoding_gzip"] in /health so the harness
runs the capture suites (android posts events to /batch and gzips them).
Also set remoteConfig = false in /init so tests don't depend on the remote
config load.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@turnipdabeets turnipdabeets marked this pull request as ready for review May 29, 2026 17:39
@turnipdabeets turnipdabeets requested a review from a team as a code owner May 29, 2026 17:39
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 29, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
sdk_compliance_adapter/src/main/kotlin/com/posthog/compliance/ComplianceAdapter.kt:352-354
**`ph.group()` fires an extra `$groupidentify` event and may trigger a second `/flags` request**

`PostHog.group()` calls `groupStateless()` internally, which calls `captureStateless(GROUP_IDENTIFY, ...)`, queuing a `$groupidentify` event. More critically, because `PostHog.reloadFeatureFlags: Boolean = true` by default, it also calls `reloadFeatureFlags(config?.onFeatureFlags)` whenever the group key is new. Since `/reset` clears persisted groups between tests, every test run sees a fresh group — making `reloadFeatureFlagsIfNewGroup` true every time. This means two `/flags` requests get made (one from `group()`, one from the explicit `reloadFeatureFlags { latch.countDown() }` below), and one `$groupidentify` batch request is created before the latch is even started. If the harness asserts exact batch or `/flags` request counts for group flag tests, those tests will remain red despite this fix.

Reviews (1): Last reviewed commit: "feat(compliance): declare capture capabi..." | Re-trigger Greptile


// Flush that event and wait so it can't spill into the next test's mock window.
ph.flush()
Thread.sleep(2000)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this? Is there a better approach, like using a latch, waiting for the flush, or something?
Sleep slows down tests

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, but tried this on iOS and it regressed 6 tests.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either tests or the mock server would have to change to make this possible
most if not all of this should be gone IMO https://github.com/search?q=repo%3APostHog%2Fposthog-android+Thread.sleep&type=code
can be a follow up ofc
once we start adding sleeps here and there, its hard to stop, take a step back, and make it in a way that we dont need it anymore

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated, no more sleep

@marandaneto
Copy link
Copy Markdown
Member

bsed on #542 (comment) that didnt work?

@marandaneto marandaneto requested a review from a team May 29, 2026 18:27
Use setGroupPropertiesForFlags(type, props, reloadFeatureFlags = false) for
group_properties (no extra reload), matching the iOS adapter pattern. group()
is still used for the type->key registration since the /flags `groups` field
reads from the GROUPS preference, which only group() writes — its
$groupidentify event and reload-on-new-group remain (no suppression param).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r-get-feature-flag

# Conflicts:
#	pnpm-workspace.yaml
@turnipdabeets turnipdabeets force-pushed the fix/compliance-adapter-get-feature-flag branch from bfcf6e8 to 0f59b4e Compare June 2, 2026 14:57
SDK.reset() reloads flags as anon (PostHog.kt:1519 — intended behavior for
real-app logout), but that /flags lands in the next test's mock window and
inflates flag counts ("Expected 0, got 1" on no_flags_request_on_init_alone
and friends). close() has no network I/O; the next /init builds a fresh
instance. Matches the iOS adapter's pattern.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@turnipdabeets turnipdabeets force-pushed the fix/compliance-adapter-get-feature-flag branch from e0fa4d5 to 639d0cd Compare June 2, 2026 23:12
turnipdabeets and others added 2 commits June 2, 2026 19:20
Per review: the remoteConfig=false in /init claimed to stop a /flags reload,
but PostHogRemoteConfig.kt:243 only chains to /flags when preloadFeatureFlags
is true — so the config does nothing observable here. Dropping it.

Also trims the capabilities and group() comments to one line each.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…pter

- pass distinct_id through via identify() so /flags carries the caller's id
- wipe the file-backed queue in /init and /reset so events don't replay across tests
- surface (not swallow) a feature-flag reload timeout
- document disable_geoip/force_remote as known SDK gaps
- extract FLUSH_SETTLE_MS / QUEUE_STORAGE_PREFIX consts, import concurrency types

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@turnipdabeets
Copy link
Copy Markdown
Contributor Author

bsed on (earlier comment) that didnt work?

For some reason I can't see this comment

@marandaneto
Copy link
Copy Markdown
Member

posthog-android Compliance Report

its the 'posthog-android Compliance Report' where all tests were still not passing, so your changes didnt have any effect
the CI job updated the comment so now at least some are passing

…d.sleep

flush() is fire-and-forget with no SDK completion callback, so /flush and
/get_feature_flag slept a fixed FLUSH_SETTLE_MS to let the batch reach the
mock. Replace the blind sleep with a Condition signalled by the tracking
interceptor once the /batch response is in hand (the mock has already
recorded the request by then): returns the instant the batch lands, capped
at FLUSH_SETTLE_MS so a never-sent flush can't hang.

Addresses Manoel's review: deterministic wait, no sleeps. Safer than the
fixed sleep since we only return after the HTTP round-trip is observed,
never earlier.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@turnipdabeets turnipdabeets force-pushed the fix/compliance-adapter-get-feature-flag branch from 0220ed3 to e1e4d6d Compare June 3, 2026 14:54
…a /flags

identify() is the only public setter for the distinctId carried in /flags,
but it unconditionally fires its own reload, adding a second /flags request
per call. That regressed 7 harness tests that assert exact /flags counts
(flags_request_*, groups_round_trip, two_flag_calls_produce_two_remote_requests,
mock_response_value_is_returned_to_caller, get_feature_flag_captures_*).

Revert to the deterministic single explicit reload. distinct_id is left
decoded-but-unapplied (same as disable_geoip/force_remote): it cannot be set
without forcing a second reload, so request_with_person_properties_device_id
stays a known public-API gap rather than breaking 7 other tests.

Verified locally against posthog-sdk-test-harness: 38/45, matching baseline
with no regression.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants